Skip to content

fix(mcp): always prompt for the MCP password on interactive http installs - #447

Merged
pcfreak30 merged 6 commits into
developfrom
feat/mcp-password-always-ask
Aug 22, 2026
Merged

fix(mcp): always prompt for the MCP password on interactive http installs#447
pcfreak30 merged 6 commits into
developfrom
feat/mcp-password-always-ask

Conversation

@pcfreak30

@pcfreak30 pcfreak30 commented Aug 22, 2026

Copy link
Copy Markdown
Member

An interactive pinner mcp install --transport http could silently skip asking the operator for the shared auth token (the MCP password) when one was already sourced from MCP_AUTH_TOKEN env, a flag, or the tunnel collector — writing agent config with a credential the user never chose or saw.

Adds an explicit, always-run "MCP Password" step for http/remote installs in interactive mode: it shows whether a token already protects the endpoint, lets the operator keep it (Enter) or type a replacement (masked, never echoed), and fails if a public endpoint would be written with no password. Non-interactive installs still source the token from flags/env without prompting.


Summary

This pull request fixes the MCP installation wizard so that it always prompts the operator for the MCP password (the shared auth token protecting a public HTTP endpoint) during interactive HTTP installs, even when a token was already inherited from environment variables, flags, or the tunnel collection step.

Key Changes

1. Always prompt for MCP password in interactive HTTP installs

  • Added a new SetMCPPassword method to the InstallUI interface and implemented it in the PTermInstallUI (terminal UI).
  • The wizard now includes a dedicated "MCP Password" step that runs before the binary resolution step for interactive HTTP installs.
  • When an auth token already exists (inherited from env/flags/tunnel collection), the operator is shown an informational message and can either keep the existing value (by pressing Enter) or type a replacement.
  • When no token exists, a warning is displayed and the password is required — the install fails if the operator provides an empty value.

2. Skipping logic

  • The password prompt is skipped in non-interactive mode (--non-interactive), where the token is sourced from flags/env without prompting.
  • It is also skipped for non-HTTP transports (e.g., stdio) since no credential is needed there.

3. Behavior change

  • Previously, an inherited auth token could be silently written without the operator seeing or confirming it. Now, the operator is always given the chance to review and replace the credential, ensuring no secret is written without explicit awareness.

4. Tests

  • Added comprehensive test coverage:
    • Verifies the prompt always fires once in interactive HTTP installs, even with an inherited token.
    • Confirms the operator's chosen password (not the inherited token) is written to the config's Authorization header.
    • Verifies non-interactive installs skip the prompt and use the env-sourced token unchanged.
    • Ensures interactive installs fail if no password is provided and none was inherited.

…alls

An interactive http install could silently skip asking the operator for the
shared auth token (MCP password) when one was already sourced from
MCP_AUTH_TOKEN env, a flag, or the tunnel collector, writing the agent config
with a credential the user never chose or saw.

Add an explicit, always-run "MCP Password" step to the install wizard for
http/remote installs in interactive mode: it shows whether a token already
exists, lets the operator keep it (Enter) or type a replacement (masked), and
fails if a public endpoint would be written with no password. Non-interactive
installs still source the token from flags/env without prompting.
@kody-ai

This comment has been minimized.

@kody-ai

kody-ai Bot commented Aug 22, 2026

Copy link
Copy Markdown

kody code-review Kody Rules medium

SetMCPPassword calls pterm.DefaultInteractiveTextInput...Show() without a timeout or context, which could hang indefinitely during the wizard's ExecuteFunc execution. While this rule targets GORM queries specifically, any GORM calls in the surrounding code should use db.WithContext(ctx) with a timeout to prevent indefinite database operations.

Kody rule violation: Disallow GORM queries without timeout

Comment thread internal/cli/mcp_install_wizard.go
Comment thread internal/cli/mcp_install_wizard.go
Comment thread internal/cli/mcp_install_wizard.go
@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown

Code Coverage Report

Total Coverage: 51.7%

Generated from commit: eb43995
Repository: LumeWeb/pinner-cli

The MCP Password step updated only the in-memory token used for the agent
config's Authorization header. When the operator replaced the password on an
install backed by a managed service, the running endpoint kept enforcing the
inherited MCP_AUTH_TOKEN from the service env file — so the agent pointed at a
credential the server rejected. Now the new password is also persisted to the
service env file and mirrored on the service state, keeping the endpoint and
the agent config on the same credential. Keeping the existing token needs no
propagation.
@kody-ai

This comment has been minimized.

Comment thread internal/cli/mcp_install_wizard.go
Persisting the new MCP_AUTH_TOKEN to the service env file is not enough on its
own: the running endpoint only reads it at process start, so it kept enforcing
the old token while the agent config used the new one. Restart the managed MCP
service after writing the new token so the live endpoint reloads it and the
connection keeps working. The restart seam is wired only in production and only
when the install is actually backed by a managed service (--service=false,
operator-run servers, and tests skip it).
@kody-ai

This comment has been minimized.

Comment thread internal/cli/mcp_install_test.go
Comment thread internal/cli/mcp_install_wizard.go
Comment thread internal/cli/mcp_install_wizard.go
Comment thread internal/mcp/services/service_command.go Outdated
Two correctness fixes to the MCP password change path:

- Thread the caller's context through RestartManagedService (and the restart
  seam) so an interrupted interactive install (Ctrl-C) can cancel the service
  restart instead of it running on context.Background().

- Order persistAuthToken so a failed restart cannot leave a mismatch: the new
  token is written to the env file (the restarted process reads it at boot)
  and only committed to state AFTER the restart succeeds. If the restart
  fails, the env file and in-memory state are rolled back to the token the
  still-running endpoint actually enforces, so disk, memory, and the live
  endpoint agree. Covered by TestMcpInstallHTTPPasswordRestartFailureRollsBack.
@kody-ai

This comment has been minimized.

Comment thread internal/cli/mcp_install_wizard.go
Comment thread internal/cli/mcp_install_wizard.go
If the managed service restart fails, the MCP password change rolls the env
file back to the token the still-running endpoint enforces. The previous code
swallowed the restore-write error, so if the rollback itself could not be
persisted the disk file silently kept the uncommitted new password while state
used the old one. Now a failed restore write is combined into the returned
error instead of masked. Covered by
TestMcpInstallHTTPPasswordRestoreFailureSurfaced.
@kody-ai

This comment has been minimized.

projectDir := t.TempDir()

envFile := filepath.Join(t.TempDir(), "mcp.env")
if err := os.WriteFile(envFile, []byte("MCP_AUTH_TOKEN=inherited-token\n"), 0o600); err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules low

Hard-coded secret value 'inherited-token' assigned to MCP_AUTH_TOKEN in mcp_install_test.go violates the rule banning secrets in Go source. Source the token from an environment variable (e.g., os.Getenv) or a secure keystore instead of embedding it directly in test code.

Kody rule violation: Ban hard-coded secrets in Go source

Prompt for LLM

File internal/cli/mcp_install_test.go:

Line 1714:

Hard-coded secret value 'inherited-token' assigned to MCP_AUTH_TOKEN in mcp_install_test.go violates the rule banning secrets in Go source. Source the token from an environment variable (e.g., os.Getenv) or a secure keystore instead of embedding it directly in test code.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

TestMcpInstallHTTPPasswordRestoreFailureSurfaced forces a failed rollback
write by making the env file's directory unwritable (os.Chmod on a dir), which
Windows does not honor, so on Windows/arm64 the restore write succeeded and the
test failed expecting the surfaced restore error. Skip the dir-permission
trigger on Windows; the rollback/restore-failure surfacing itself is
OS-independent and remains covered on all platforms by
TestMcpInstallHTTPPasswordRestartFailureRollsBack.
@kody-ai

kody-ai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Kody Review Complete

Great news! 🎉
No issues were found that match your current review configurations.

Keep up the excellent work! 🚀

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

  • Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug
Performance
Security
Business Logic

Access your configuration settings here.

@pcfreak30
pcfreak30 merged commit 8977968 into develop Aug 22, 2026
14 checks passed
@pcfreak30
pcfreak30 deleted the feat/mcp-password-always-ask branch August 23, 2026 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant